fix: blur should not trigger close when next activeElement still is input ref #1175
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough移除 BaseSelect 的模糊时立即懒关闭调用；在 SelectInput 的 blur 处理中使用由 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Input as SelectInput
participant MacroTask as macroTask (useOpen)
participant OpenHook as toggleOpen / useOpen
User->>Input: 触发 blur 事件
activate Input
Input->>MacroTask: 注册宏任务延迟执行检查
deactivate Input
Note over MacroTask: 等待当前微/宏任务完成（延后执行）
MacroTask->>Input: 运行延迟回调 -> 检查 document.activeElement
alt 输入仍为活跃或包含活跃元素
Note right of Input: 跳过关闭
else 输入非活跃
Input->>OpenHook: 调用 toggleOpen(false)
OpenHook->>OpenHook: 更新 open 状态
end
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟 需要额外关注的文件/点：
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @zombieJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in Ant Design's select components where the dropdown would close unexpectedly when the input field lost focus, even if the focus was still within a related element of the component. The fix introduces a more robust blur handling mechanism that defers the decision to close the dropdown until it's confirmed that the focus has truly left the component, thereby improving the user experience and preventing premature closures. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1175 +/- ##
=======================================
Coverage 99.41% 99.41%
=======================================
Files 31 31
Lines 1196 1200 +4
Branches 425 428 +3
=======================================
+ Hits 1189 1193 +4
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves an issue where the select dropdown would close prematurely on blur. The solution, which involves deferring the close logic using a macroTask to check the document.activeElement after focus has shifted, is a solid approach. The related refactoring in useOpen to always handle closing asynchronously simplifies the logic. The test updates are also good, as they more accurately simulate real browser behavior for blur events. I have one minor suggestion to remove some commented-out code to keep the codebase clean.
src/BaseSelect/index.tsx
Outdated
| // triggerOpen(false, { | ||
| // lazy: true, | ||
| // }); |
There was a problem hiding this comment.
This commented-out code is no longer necessary since the logic for closing the dropdown on blur has been moved to the SelectInput component. It should be removed to improve code clarity.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/BaseSelect/index.tsx (1)
557-578: 建议移除注释代码或添加说明。
triggerOpen(false, { lazy: true })调用被注释掉，因为 blur 关闭逻辑已移至SelectInput中处理。这是正确的架构调整，但代码库中留下注释代码会降低可维护性。建议：
- 直接移除注释代码（首选），或
- 添加详细的 TODO 注释说明为何保留，例如：
const onInternalBlur: React.FocusEventHandler<HTMLElement> = (event) => { setFocused(false); if (mergedSearchValue) { // `tags` mode should move `searchValue` into values if (mode === 'tags') { onSearch(mergedSearchValue, { source: 'submit' }); } else if (mode === 'multiple') { // `multiple` mode only clean the search value but not trigger event onSearch('', { source: 'blur', }); } } if (!disabled) { - // triggerOpen(false, { - // lazy: true, - // }); + // Note: Blur close logic moved to SelectInput to check activeElement before closing + // This prevents closing when focus moves to another input within the component onBlur?.(event); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/BaseSelect/index.tsx(1 hunks)src/SelectInput/index.tsx(2 hunks)src/hooks/useOpen.ts(2 hunks)tests/Combobox.test.tsx(2 hunks)tests/Select.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/SelectInput/index.tsx (2)
src/hooks/useOpen.ts (1)
macroTask(10-19)tests/utils/common.ts (1)
toggleOpen(22-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
src/hooks/useOpen.ts (4)
10-19: 导出 macroTask 工具函数是合理的。将
macroTask导出使得其他模块（如SelectInput）可以使用延迟执行机制来处理异步逻辑，这与 PR 修复 blur 关闭逻辑的目标一致。
70-76: 解构更新与类型定义保持一致。移除
lazy参数的解构与TriggerOpenType的类型更新一致。
79-93: 简化的toggleOpen逻辑符合新的 blur 处理机制。移除
lazy条件后，关闭操作始终通过macroTask延迟执行，这允许在SelectInput的 blur 处理中正确检查document.activeElement，确保焦点仍在输入框内时不会关闭下拉框。
21-31: The review comment is incorrect—lazywas not removed from the type definition.The
lazyproperty still exists inTriggerOpenTypeat line 29 ofsrc/hooks/useOpen.ts. While it was never actually implemented (the only reference is commented-out code insrc/BaseSelect/index.tsxline 574), it remains in the type signature. This is dead code that should be cleaned up, not a breaking change being introduced. Sincelazywas never functional, no external consumers could be depending on it.Likely an incorrect or invalid review comment.
src/SelectInput/index.tsx (1)
14-14: 导入macroTask用于延迟 blur 处理。从
useOpen导入macroTask工具函数，用于实现下方的延迟关闭逻辑。tests/Select.test.tsx (1)
808-815: 测试更新正确模拟了新的 blur 行为。显式调用
input.blur()并在act块中运行所有定时器，确保 blur 处理逻辑（包括macroTask延迟的关闭检查）在断言之前完成。这使测试更具确定性，并正确验证了 blur 不会在焦点保持时关闭下拉框的修复。tests/Combobox.test.tsx (2)
505-510: 测试正确模拟了 blur 事件处理。显式调用
input.blur()确保浏览器的activeElement状态更新，随后的delay(100)等待macroTask延迟的关闭逻辑执行完成。
594-599: 测试更新与其他测试文件保持一致。在
act块中调用input.blur()和jest.runAllTimers()的模式与Select.test.tsx中的更新一致，确保 blur 处理逻辑完整执行后再进行断言。
| const onInternalBlur: SelectInputProps['onBlur'] = (event) => { | ||
| toggleOpen(false); | ||
| macroTask(() => { | ||
| const inputNode = getDOM(inputRef.current); | ||
| if (inputNode !== document.activeElement && !inputNode.contains(document.activeElement)) { | ||
| toggleOpen(false); | ||
| } | ||
| }); | ||
|
|
||
| onBlur?.(event); | ||
| }; |
There was a problem hiding this comment.
blur 处理逻辑修复了焦点转移问题，但需要添加空值检查。
使用 macroTask 延迟关闭逻辑确保在 document.activeElement 更新后再检查焦点位置，这正确修复了当焦点转移到同一组件内的其他输入元素时错误关闭下拉框的问题。
但是，getDOM(inputRef.current) 可能返回 null，应添加空值检查以避免潜在的运行时错误。
应用以下 diff 添加空值检查：
const onInternalBlur: SelectInputProps['onBlur'] = (event) => {
macroTask(() => {
const inputNode = getDOM(inputRef.current);
- if (inputNode !== document.activeElement && !inputNode.contains(document.activeElement)) {
+ if (inputNode && inputNode !== document.activeElement && !inputNode.contains(document.activeElement)) {
toggleOpen(false);
}
});
onBlur?.(event);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onInternalBlur: SelectInputProps['onBlur'] = (event) => { | |
| toggleOpen(false); | |
| macroTask(() => { | |
| const inputNode = getDOM(inputRef.current); | |
| if (inputNode !== document.activeElement && !inputNode.contains(document.activeElement)) { | |
| toggleOpen(false); | |
| } | |
| }); | |
| onBlur?.(event); | |
| }; | |
| const onInternalBlur: SelectInputProps['onBlur'] = (event) => { | |
| macroTask(() => { | |
| const inputNode = getDOM(inputRef.current); | |
| if (inputNode && inputNode !== document.activeElement && !inputNode.contains(document.activeElement)) { | |
| toggleOpen(false); | |
| } | |
| }); | |
| onBlur?.(event); | |
| }; |
🤖 Prompt for AI Agents
In src/SelectInput/index.tsx around lines 200 to 209, the blur handler uses
getDOM(inputRef.current) without checking for null which can cause a runtime
error; update the macroTask callback to first check that inputNode is not null
(or undefined) before comparing it to document.activeElement or calling
inputNode.contains(...), and only call toggleOpen(false) when inputNode exists
and the activeElement is outside it; keep the existing onBlur?.(event) call
unchanged.
ref ant-design/ant-design#55928
Summary by CodeRabbit
Bug Fixes（缺陷修复）
Refactor（重构）
Tests（测试）
✏️ Tip: You can customize this high-level summary in your review settings.